Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Require sb3 version 2 or newer #148

Merged
merged 9 commits into from
Oct 3, 2023
Merged

Require sb3 version 2 or newer #148

merged 9 commits into from
Oct 3, 2023

Conversation

Ivan-267
Copy link
Collaborator

@Ivan-267 Ivan-267 commented Sep 22, 2023 •
edited
Loading

Our environment will not work with older sb3 versions, due to using gymnasium (where older sb3 versions used gym).

Edit: This may cause an issue with rllib, I will check the test results and see if it can be addressed if it does.

@Ivan-267
Require sb3 version 2 or newer
031bde6 
Our environment will not work with older sb3 versions, due to using gymnasium (where older sb3 versions used gym).
@Ivan-267
Install rllib without reinstalling older sb3
f59fc2c 
This may fix the related error in tests. However, it will now install rllib separately from what is set in .[rllib].
@Ivan-267
Attempt to fix the execution provider error
29c78e7
edbeeching
edbeeching reviewed Sep 23, 2023
View reviewed changes
.github/workflows/test-ci.yml
@@ -77,7 +77,7 @@ jobs:
- name: Clean up dependencies
run: |
pip uninstall -y stable-baselines3 gymnasium
pip install .[rllib]
pip install ray[rllib]
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think rllib requires gymnasium==0.26.3 which I why we install our .[rllib] deps, see:

godot_rl_agents/setup.cfg

Line 50 in 00ddb9e

rllib =

edbeeching
edbeeching approved these changes Sep 23, 2023
View reviewed changes
Copy link
Owner

@edbeeching edbeeching left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM apart from the question about rllib deps

@Ivan-267
Copy link
Collaborator Author

Ivan-267 commented Sep 23, 2023 •
edited
Loading

It seems that older Ray is installed on the tests that pass, so by specifying the gymnasium version we could be enforcing older Ray (which works with our config) to be installed. On the newer version of Ray, I also locally had some issues unless I try a different path format. Even outside of the path issue, I also had some issues with one of my environments with both Ray versions, something about the format of observations, but I didn't test it in-depth, so I don't know the issue was related to my Godot environment (which worked with SB3 which I mostly use for now), or something else.

I'll add the gymnasium version as a quick temporary fix, if it works, we can merge the PR for now since it's focused on ensuring the correct SB3 version, and in the future we can test our Ray configuration and see if something needs to be updated.

Edit: Still newer gymnasium with ray-2.7.0 is being installed, I'll have to look into this a bit more later.

@Ivan-267
Copy link
Collaborator Author

With this approach, newer Ray 2.7 seems to work, although I didn't perform much testing to see if everything works with this as before. As a quick fix I've changed the folder path to an absolute path as that seems to fix the error. An alternative could be to use older Ray or find a different fix for the issue.

It should be noted that one of my environments didn't work with rllib (some observation type error), but I think that holds for both Ray versions, and may need more troubleshooting at some future point as I'm not sure about the cause yet (maybe something with the env itself even though it works with SB3). Jumperhard worked on my PC as well, but there might be some difference in observations from my env (which has some floats and a raycast obs array).

For future reference and in case similar errors appear in the future, the error was:

ValueError: The two structures don't have the same nested structure.

First structure: type=dict str={'obs': [0.0028, 0.000441726500866935, 0.00281361560337245, 0.00664886436425149, 0.260605245828629, 0.39156648516655, -0.00112051225733, -1.99999463558197, 0.781054401397705, 0.746744704246521, 0.712950587272644, 0.76171236038208, 0.769729685783386, 0.743742609024048, 0.659272933006287, 0.363503074645996, 0, 0.292076635360718, 0.415652275085449, 0.244122076034546, 0, 0.216817808151245, 0.643476295471191, 0.747096061706543, 0.781146144866943]}

Second structure: type=OrderedDict str=OrderedDict([('obs', array([-0.19811498,  0.8486953 ,  0.7596289 ,  0.2623617 , -0.08332985,
        0.21301392,  0.7458078 , -0.4820755 , -0.88564146, -0.6634987 ,
        0.82966214,  0.6683578 , -0.18554112, -0.7013975 ,  0.8797588 ,
       -0.24309783,  0.77364075,  0.4879668 , -0.982385  , -0.4481921 ,
        0.8362797 , -0.21034704,  0.9531232 , -0.33154443,  0.886062  ],
      dtype=float32))])

More specifically: Substructure "type=list str=[0.0028, 0.000441726500866935, 0.00281361560337245, 0.00664886436425149, 0.260605245828629, 0.39156648516655, -0.00112051225733, -1.99999463558197, 0.781054401397705, 0.746744704246521, 0.712950587272644, 0.76171236038208, 0.769729685783386, 0.743742609024048, 0.659272933006287, 0.363503074645996, 0, 0.292076635360718, 0.415652275085449, 0.244122076034546, 0, 0.216817808151245, 0.643476295471191, 0.747096061706543, 0.781146144866943]" is a sequence, while substructure "type=ndarray str=[-0.19811498  0.8486953   0.7596289   0.2623617  -0.08332985  0.21301392
  0.7458078  -0.4820755  -0.88564146 -0.6634987   0.82966214  0.6683578
 -0.18554112 -0.7013975   0.8797588  -0.24309783  0.77364075  0.4879668
 -0.982385   -0.4481921   0.8362797  -0.21034704  0.9531232  -0.33154443
  0.886062  ]" is not
Entire first structure:
{'obs': [., ., ., ., ., ., ., ., ., ., ., ., ., ., ., ., ., ., ., ., ., ., ., ., .]}
Entire second structure:
OrderedDict([('obs', .)])

@Ivan-267 Ivan-267 requested a review from edbeeching September 23, 2023 12:41
@Ivan-267 Ivan-267 mentioned this pull request Oct 3, 2023
Merged
@edbeeching edbeeching merged commit 7935627 into main Oct 3, 2023
12 checks passed
@edbeeching edbeeching deleted the install-sb3-v2-or-newer branch October 3, 2023 16:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@edbeeching edbeeching Awaiting requested review from edbeeching

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Ivan-267 @edbeeching